Jab bdms 626#607
Merged
ksmuczynski merged 6 commits intokas-well-BDMS-626-inventory-ingestion-updates_v2from Mar 18, 2026
Merged
Jab bdms 626#607ksmuczynski merged 6 commits intokas-well-BDMS-626-inventory-ingestion-updates_v2from
ksmuczynski merged 6 commits intokas-well-BDMS-626-inventory-ingestion-updates_v2from
Conversation
Each contact should have a role and contact_type
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens contact validation across the well-inventory CSV import and contact models by requiring role and contact_type whenever contact data is supplied, aligning schema and DB constraints and updating tests/BDD scenarios accordingly.
Changes:
- Enforce
Contact.roleandContact.contact_typeas non-nullable in DB/model and schema response/create types. - Update well-inventory row validation to require contact role/type when any contact data is present.
- Update unit tests, BDD feature scenarios/steps, and CSV fixtures to reflect the stricter validation and ensure BDD tests target
ocotilloapi_test.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
services/well_inventory_csv.py |
Adjusts contact payload creation for CSV imports (but introduces a likely Enum/string mismatch for DB inserts). |
schemas/well_inventory.py |
Adds cross-field validation requiring contact role/type when contact data is present. |
schemas/contact.py |
Makes role and contact_type required in request/response schemas. |
db/contact.py |
Makes role and contact_type non-nullable at the ORM level. |
tests/test_well_inventory.py |
Updates expectations for missing contact role/type to fail. |
tests/features/well-inventory-csv.feature |
Switches “missing role/type” scenarios from positive to negative and asserts validation errors. |
tests/features/steps/well-inventory-csv-validation-error.py |
Adds/updates Behave step assertions for the new validation messages (includes a duplicate function name). |
tests/features/steps/cli_common.py |
Improves assertion message for non-zero exit codes. |
tests/features/environment.py |
Forces BDD tests onto the ocotilloapi_test DB (currently hardcodes env overrides). |
tests/features/data/*.csv |
Updates fixtures to include unique names and represent missing role/type cases. |
alembic/versions/*.py (deleted) |
Removes migrations that made contact role/type nullable. |
You can also share your feedback on Copilot code review. Take the survey.
ksmuczynski
approved these changes
Mar 18, 2026
Contributor
ksmuczynski
left a comment
ksmuczynski
left a comment
There was a problem hiding this comment.
Looks good! Approved
ksmuczynski
merged commit bf65262
into
kas-well-BDMS-626-inventory-ingestion-updates_v2
5 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
This PR addresses the following problem / context:
contactshould have a definedroleandcontact_typeto give them context. Furthermore, there are no records in the csv where there are no defined roles and typesHow
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?
ocotilloapi_testis used for feature tests